Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Derive Clone on Parent and Children #15574

Closed
wants to merge 2 commits into from
Closed

Derive Clone on Parent and Children #15574

wants to merge 2 commits into from

Conversation

atornity
Copy link
Contributor

@atornity atornity commented Oct 1, 2024

Objective

Parent and Children should be Cloneble.

Solution

Derive Clone on Parent and Children.

@alice-i-cecile alice-i-cecile added the X-Controversial There is active debate or serious implications around merging this PR label Oct 1, 2024
@MrGVSV
Copy link
Member

MrGVSV commented Oct 1, 2024 •
edited
Loading

Wouldn't this allow users to accidentally put these components into invalid states?

For example, cloning Parent and inserting it onto an entity that isn't one of its children.

@alice-i-cecile
Copy link
Member

This enables UB via mem swap shenanigans breaking the invariants relied on during unsafe transform propagation. So we can't merge this.

@alice-i-cecile alice-i-cecile added D-Trivial Nice and easy! A great choice to get started with Bevy C-Usability A targeted quality-of-life change that makes Bevy easier to use A-Hierarchy Parent-child entity hierarchies labels Oct 1, 2024
@atornity atornity deleted the parent-clone branch October 1, 2024 20:04
@alice-i-cecile alice-i-cecile mentioned this pull request Oct 1, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
A-Hierarchy Parent-child entity hierarchies C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Trivial Nice and easy! A great choice to get started with Bevy X-Controversial There is active debate or serious implications around merging this PR
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@atornity @MrGVSV @alice-i-cecile